fix: soilweb links — cokey-based URL mapping + deterministic ordering - #373
Merged
Conversation
… (cherry-pick #340) Cherry-picked the code changes from PR #340 (fix/soilweb-links) onto current main. - us_soil.py: replace index-based SDE/SEE URL storage with a cokey->URL map so components get the URL for the right series after reordering; deterministic groupby/sort ordering for component processing. - utils.py: texture-name normalization, getTexture returns None on NaN inputs (+ Silty clay / Silty clay loam classes), information_gain weighted-entropy fix. 3-way merged with main's drop_cokey_horz distance/cokey tiebreaker. - services.py: minor supporting change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Regenerated against the pinned CI DB image (ghcr.io/techmatters/soil-id-db:2026-06-23) inside a GDAL-capable container. 27 snapshots updated (16 US, 11 global), 3 unchanged. Verified deterministic: a second run without --snapshot-update passes clean (30/30). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Regenerating the unit-test output snapshots was a fiddly manual process: GDAL won't build under uv/pip on macOS, and the snapshots are environment-sensitive, so they must be produced against an interpreter matching CI and the exact pinned soil-id-db image CI uses (not the local :latest terraso-backend runs). Automate it: - scripts/snapshot-runner.Dockerfile: CI-faithful runner image (Ubuntu + ubuntugis + Python 3.13 + pinned requirements; GDAL 3.11.4). - scripts/run_in_container.sh: provisions the pinned DB on a private network (no host port, so it can't collide with a running terraso-backend on 5432) and runs given make targets in the runner with the repo bind-mounted. - scripts/regen_snapshots.sh: regenerate + verify determinism + git status. - Makefile: test / test_unit / test_update_unit_snapshots auto-detect when GDAL isn't importable and transparently re-run themselves in the container (NATIVE=1 forces native and prevents recursion), so the same make command works on macOS and CI. Adds a regen_snapshots target. - Document the workflow in README.md and scripts/README.md. Verified: make test_update_unit_snapshots on macOS containerizes and reproduces the committed snapshots byte-for-byte, and the verify pass confirms determinism. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…lity The auto-detect guard checked only whether *some* python3 could import GDAL. On macOS a Homebrew python3 (brew install gdal) imports GDAL but has no pytest, while the uv venv has pytest but no GDAL — so `make test_update_unit_snapshots` took the native path and then failed on a missing pytest, instead of containerizing. Gate the native path on the interpreter having pytest AND the *pinned* GDAL version (requirements.txt), run tests via `$(PYTHON) -m pytest` so the check and execution share an interpreter, and expose PYTHON as an override. This also closes a correctness hole: snapshots are GDAL-version-sensitive, so a native GDAL of a different version must not silently produce snapshots that disagree with CI — it's routed to the pinned-version container instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
knipec
approved these changes
Jul 16, 2026
knipec
left a comment
Contributor
There was a problem hiding this comment.
Reviewed only from a very high level:
- Johannes talked thru the "mac has an issue with a GDAL version mismatch that prevents us from running the tests, so if that's the case run the tests via docker so it has the right GDAL version".
- Didn't look at the code much, and don't have much of an understanding of the soil id algorithm changes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rebased, cherry-picked replacement for #340 (which was stale and conflicting with
main).What this does
Fixes incorrect SDE/SEE (SoilWeb) URL mapping in US soil identification, where components received URLs intended for other components due to index misalignment after component reordering.
us_soil.py— replace index-based SDE/SEE URL storage with acokey->URL map, so each component gets the URL for the right series after reordering; deterministicgroupby/sort ordering for component processing.utils.py— texture-name normalization,getTexturereturnsNoneon NaN inputs (+Silty clay/Silty clay loamclasses),information_gainweighted-entropy fix. 3-way merged with main'sdrop_cokey_horzdistance/cokey tiebreaker.services.py— minor supporting change.Snapshots
Regenerated against the pinned CI DB image (
ghcr.io/techmatters/soil-id-db:2026-06-23): 27 updated (16 US, 11 global), 3 unchanged. Verified deterministic — a second run without--snapshot-updatepasses clean (30/30).Example of the fix: a component that previously showed
sde/see?series=balcomnow correctly maps tolos_osos.Note
US tests make live USDA SDA calls for ESD/ecosite data (not mocked); those fields depend on the live service and are the most likely source of any future snapshot drift.
Supersedes #340.
🤖 Generated with Claude Code